Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Key API and keys event handling #556

Merged
merged 6 commits into from
Dec 28, 2021
Merged

Conversation

ghost
Copy link

@ghost ghost commented Dec 20, 2021

This PR improves the whole key event handling API by the following aspects:

  • improved Key-class based upon a new concept of Modifier-interface, which enables constructing nice key shortcut combinations like "Strg + K" or "Shift + Alt + F" or alike.
  • add some convenience methods to improve the event handling for KeyboardEvents: keys()
  • improved Keys-object with predefined common keys like Tab, Enter but also the modifier keys like Alt, Shift, Meta and Control
  • mark some special interest functions as deprecated in favour of using the new keys functions, which does the heavy work without being too specialized.

The new key API allows easily defining combinations of keys with modifier keys, constructing those from a KeyboardEvent and prevents meaningless combinations of "real" keys:

// constructing a key by hand
Key("K") // will convert "K" automatically to lower case -> Key(name = "k", ctrl = false, alt = false, shift = false, meta = false)
// same as
Key("k")
// set modifier states
Key("K", ctrl = true) // Key(name = "k", ctrl = true, alt = false, shift = false, meta = false)

// constructing a key from a KeyboardEvent
div {
    keydowns.map { Key(it) } handledBy { /* use Key-object for further processing */ }
    //                 ^^
    //                 use KeyboardEvent to construct a Key-object with all potentially modifier key states reflected!
}

// using predefined keys from Keys object
Keys.Enter // "real" Key
Keys.Alt // ModifierKey -> needs to be combined with a "real" key in order to use it for further processing
// the same but more cumbersome and open to typos
Key("Enter")
// not the same (!)
Key("Alt") // -> Key(name = "Alt", ..., alt = false)
Keys.Alt // -> ModifierKey-object with alt = true property!

// constructing a key with some modifier key
Key("K") + Keys.Control
// same result, but much more readable the other way round:
Keys.Control + "K"

// defining some common combination:
val searchKey = Keys.Control + Keys.Shift + "F"
//              ^^^^^^^^^^^^
//              You can start with a modifier key!
//              Appending a String to an ModifierKey will finally lead to a Key

val tabbing = setOf(Keys.Tab, Keys.Shift + Keys.Tab)

// API prevents accidently usage:
// won't compile!
Key("F") + Key("P") // cannot combine real keys!

As most of the time you will use keys within the handling of some KeyboardEvent, there are some new methods for leveraging some common work by the framework. Those keys-methods variants accept one, arbitrary or a set of Keys which should be handled. All events from other keys will be ignored, so that one can prevent default behaviour or stop bubbling explicitly only for the handled keys. The functions return a pair of the key and the corresponding event, so that calling stopPropagation or accessing other properties of the event is still possible for the client.

Example usage:

// within some Tag<*or WithDomNode<*>

// do something only for some key shortcut combination
keydowns.keys(Keys.Strg + "K").map { } handledBy { /* popup some command entry component */ }

// accessing the event after the filtering:
keydowns.keys(Keys.ArrowUp, Keys.ArrowDown).mapNotNull { (key, e) ->
    e.preventDefault() // page should not scroll up or down in this case!
    when (key) {
        Keys.ArrowDown -> // create / modify something to be handled
        Keys.ArrowUp -> // create / modify something to be handled
        else -> null
    }
} handledBy { /* ... */ }

@ghost ghost requested review from jamowei and jwstegemann December 20, 2021 16:20
@ghost ghost self-assigned this Dec 20, 2021
@ghost ghost added api breaking Forces client sources to change api improvement enhancement New feature or request labels Dec 20, 2021
@ghost ghost added this to the 0.14 milestone Dec 20, 2021
@ghost ghost mentioned this pull request Dec 20, 2021
@ghost ghost removed the api breaking Forces client sources to change label Dec 20, 2021
@ghost
Copy link
Author

ghost commented Dec 20, 2021

Question: Would it be nice from the Key ctor to call a lowercase on all Strings? A client might be trapped by the current strict behaviour to define some Keys.Control + "F" with an uppercase "F". But this will never ever be recognized by the keys functions as the DOM API only emits lowercase characters... to achieve a reaction to a "STRG + K" combination the key must be constructed with a lower case character right now: Keys.Control + "f".

On the other hand it would be at least one call on top and also would probably need some sort of filter to only apply the lowercase to one length strings (as the predefined all starts with upper case!)

@ghost
Copy link
Author

ghost commented Dec 21, 2021

After dicussing the issue with lower- vs upper-case keys, we have decided to look for a rational implementation that will detect all none special keys and transform them to lower-cases. This way the API is much more pleasent as it would make no difference if one writes Keys.Control + "K" of Keys.Control + "k" or even Key("K") vs Key("k").

@ghost ghost marked this pull request as draft December 21, 2021 11:18
Christian Hausknecht and others added 5 commits December 21, 2021 17:37
- add some convenience methods to improve the event handling for `KeyboardEvent`s: ``keys()`` which accepts one, arbitrary or a set of `Key`s which should be handled. All events from other keys will be dropped, so that one can prevent default behaviour or stop bubbling explicitly only for the handled keys
- returns a pair of the key and the accompanied event, so that calling `stopPropagation` or accessing other properties of the event is still possible for the client.
- simplify predefined keys to be just string constants. This reduces object construction overhead for static keys including missing event.
- reshape `Key` class to expose the relevant fields for a key as properties combined with the corresponding `KeyboardEvent` in order to process the event further without loosing those informations.
- add `keys()` functions for `WindowListener` too
- mark `key()` function as deprecated as there is really no use case for this! (Grabbing a key without filtering just makes no sense at all!)
- adapt some components to new API and `keys`-functions.
- adapt test cases
- add concept of "extra keys" with own interface. This enables combination with other extra keys and "real" keys but prevents combination of real keys, which would be meaningless:
```kotlin
Keys.Control + "K"
Key("K", ctrl = true)
Keys.Control + Keys.Shift + "F"
Key("F") + Keys.Alt
// won't compile
Key("F") + Key("P")
```
- offer extra keys as predefined objects in ``Keys``-object
- rework `keys`-functions to work with new Key-API
- deprecate some key event based special interest functions in favour of new `keys` functions, which leverages the heavy work already in a generic way
@ghost ghost force-pushed the chausknecht/newKeyApi branch from 9e61485 to a060cad Compare December 21, 2021 16:39
@ghost ghost marked this pull request as ready for review December 21, 2021 16:40
@ghost ghost force-pushed the chausknecht/newKeyApi branch from a060cad to ea11428 Compare December 21, 2021 17:17
- ctor of `Key` now automatically converts single letter key names to lower-case in order to be compliant to keyboard-events key property, which is always lower-case for single letter keys.
- add unit tests
- adapt documentation
@jamowei jamowei merged commit b5f155f into master Dec 28, 2021
@ghost ghost deleted the chausknecht/newKeyApi branch January 3, 2022 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api improvement enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant